feat(cli): runs export + invoke --scaffold + envelope contract test vs cloud (#189)#192
Conversation
…s cloud (#189) P2 of #189, consuming cloud#1841's envelope capture: - `agentworkforce runs export <runId> [--fixture event.json] [--agent <selector>]` — pulls the byte-exact envelope cloud delivered to a run from GET /runs/:runId/envelope and writes it as the single-envelope fixture `invoke` consumes. The endpoint is agent-scoped, so without --agent every workspace agent is probed for the run id (bounded). Omitted (oversized) and pre-capture runs get user-actionable errors pointing at --scaffold; a contract-violating captured:true/null response is refused, never fabricated (interpretEnvelopeResponse, exported + unit-tested). Reuses the deployments list/logs auth + fetch helpers (now exported from list-command). - `agentworkforce invoke --scaffold <type>` — cold-start fixture skeleton: frame filled (name/cron for cron.tick), `resource` left as an explicit TODO hole for provider events (its shape is decided by adapter normalization + cloud buildEnvelope and shouldn't be guessed). <type> validated against KNOWN_TRIGGER_CATALOG with the lintTriggers warn-don't-block stance. - Envelope contract vs cloud: RawGatewayEnvelope gains the 5 fields cloud's buildEnvelope emits that the type lacked (provider, eventType, deliveryId, paths, resumeContext) + a RAW_GATEWAY_ENVELOPE_FIELDS const compile-checked against the type (`satisfies keyof`). envelope-fields.cloud.ts is the checked-in copy of cloud's ENVELOPE_FIELDS (source + update procedure in the header); shim.contract.test.ts asserts every cloud field is documented and that a full cloud-shaped envelope still shims/dispatches — drift now fails CI on whichever side moves first (cloud's pin test points here). - README: fixture-provenance section (export-real-fire as the primary path, scaffold for cold start, omit-never-truncate noted). Tests: runtime 52/52 (3 new contract), CLI 240/240 (11 new: parse, captured/omitted/pre-capture/violating-response contracts, scaffold frame/TODO-hole/warn-don't-block/e2e). Full `pnpm run check` green. Refs #189, AgentWorkforce/cloud#1841 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
CodeAnt AI is reviewing your PR. |
|
Warning Review limit reached
More reviews will be available in 51 minutes and 31 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds fixture sourcing and scaffold tooling: a new ChangesFixture Export and Scaffold Workflow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeant-ai
Bot
added
the
size:XL
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the runs export command to retrieve gateway envelopes from cloud runs as replayable local fixtures, alongside a --scaffold option for generating skeleton fixtures. It also establishes a cross-repo contract test to prevent drift in envelope fields. Feedback on these changes suggests refining the CLI argument parser to support inline values for --workspace and --cloud-url, and modifying the agent probe's error handling to only ignore 404 errors rather than silently swallowing all exceptions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| } catch { | ||
| // 404 = not this agent's run (or auth/transient on this probe); | ||
| // keep scanning the remaining candidates. | ||
| continue; | ||
| } |
There was a problem hiding this comment.
The catch block silently swallows all errors during the agent runs probe. While skipping 404 Not Found errors is expected when scanning multiple agents, swallowing other critical errors (such as 401 Unauthorized, 500 Internal Server Error, or network timeouts) will hide the root cause and mislead the user into thinking the run ID simply does not exist. Only 404 errors should be caught and ignored to continue the scan.
| } catch { | |
| // 404 = not this agent's run (or auth/transient on this probe); | |
| // keep scanning the remaining candidates. | |
| continue; | |
| } | |
| } catch (err) { | |
| // Only swallow 404 errors to continue scanning other candidates. | |
| // Propagate other critical errors (e.g., 401, 500, network issues) immediately. | |
| if (err instanceof Error && err.message.includes('failed: 404')) { | |
| continue; | |
| } | |
| throw err; | |
| } |
| } else if (a === '--workspace') { | ||
| workspace = expectValue('--workspace', rest[++i]); | ||
| } else if (a === '--cloud-url') { | ||
| cloudUrl = expectValue('--cloud-url', rest[++i]); |
There was a problem hiding this comment.
The parser for runs export does not support inline values for --workspace and --cloud-url (e.g., --workspace=my-workspace or --cloud-url=http://...), which is inconsistent with other CLI commands and will cause the CLI to throw an 'unknown flag' error. Adding support for inline values ensures consistency and prevents unexpected crashes.
| } else if (a === '--workspace') { | |
| workspace = expectValue('--workspace', rest[++i]); | |
| } else if (a === '--cloud-url') { | |
| cloudUrl = expectValue('--cloud-url', rest[++i]); | |
| } else if (a === '--workspace') { | |
| workspace = expectValue('--workspace', rest[++i]); | |
| } else if (a.startsWith('--workspace=')) { | |
| workspace = expectInline('--workspace', a.slice('--workspace='.length)); | |
| } else if (a === '--cloud-url') { | |
| cloudUrl = expectValue('--cloud-url', rest[++i]); | |
| } else if (a.startsWith('--cloud-url=')) { | |
| cloudUrl = expectInline('--cloud-url', a.slice('--cloud-url='.length)); |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| } catch { | ||
| // 404 = not this agent's run (or auth/transient on this probe); | ||
| // keep scanning the remaining candidates. | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| if (!payload || !matchedAgent) { | ||
| throw new Error( | ||
| `run ${opts.runId} not found in workspace ${ctx.workspace}` + | ||
| `${opts.agent ? ` for agent "${opts.agent}"` : ` across ${candidates.length} agent(s)`}. ` + | ||
| 'Check the run id (dashboard run detail, or `agentworkforce deployments list`).' | ||
| ); |
There was a problem hiding this comment.
🟠 Architect Review — HIGH
The agent-probing loop in runRunsExport swallows all request failures from the envelope endpoint (including auth and server errors) and reports them as "run not found", so genuine cloud/API issues are misrepresented as a missing run.
Suggestion: Only continue probing on explicit not-found responses for that agent (e.g., 404), and immediately surface unauthorized/server/network failures with their original context; keep the "not found across N agents" error exclusively for the true all-404 case.
Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** packages/cli/src/runs-command.ts
**Line:** 191:203
**Comment:**
*HIGH: The agent-probing loop in `runRunsExport` swallows all request failures from the envelope endpoint (including auth and server errors) and reports them as "run not found", so genuine cloud/API issues are misrepresented as a missing run.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| id: 'evt_local_1', | ||
| workspace: 'ws-local', | ||
| type: 'cron.tick', | ||
| occurredAt, | ||
| name: 'TODO: your schedule name (persona schedules[].name)', | ||
| cron: '0 9 * * 1' | ||
| }, | ||
| warnings | ||
| }; | ||
| } | ||
|
|
||
| const firstDot = type.indexOf('.'); | ||
| const provider = firstDot > 0 ? type.slice(0, firstDot) : ''; |
There was a problem hiding this comment.
Suggestion: Any cron.* scaffold request is silently rewritten to cron.tick, so the generated fixture can represent a different event type than the user asked for. Preserve the requested type (or emit a warning/error) instead of coercing it. [incorrect variable usage]
Severity Level: Major ⚠️
- ⚠️ `invoke --scaffold` cron fixtures use incorrect type value.
- ⚠️ Mismatch between requested type and written fixture JSON.Steps of Reproduction ✅
1. Invoke the CLI in scaffold mode, e.g. run `agentworkforce invoke --scaffold
cron.tick:my-schedule` so that `runInvoke()` in
`packages/cli/src/invoke-command.ts:261-289` is called with `args = ['--scaffold',
'cron.tick:my-schedule']`.
2. Inside `parseInvokeArgs()` at `packages/cli/src/invoke-command.ts:61-125`, the loop
sets `scaffoldType = 'cron.tick:my-schedule'` and, because `scaffoldType` is truthy,
returns `{ scaffold: scaffoldType, outputPath?: ... }` at lines 107-110 without requiring
a persona or fixture.
3. Back in `runInvoke()` at `packages/cli/src/invoke-command.ts:273-288`, the `'scaffold'
in opts` branch calls `scaffoldFixture(opts.scaffold)` with the original type string
`'cron.tick:my-schedule'` and then writes the returned `fixture` JSON to stdout or
`--output`.
4. In `scaffoldFixture()` at `packages/cli/src/invoke-command.ts:12-31` (diff lines
419-431), the condition `if (type === 'cron.tick' || type.startsWith('cron.'))` matches
`'cron.tick:my-schedule'`, but the constructed fixture hardcodes `"type": "cron.tick"`
instead of the requested `type` value, so the generated envelope's `type` field no longer
matches the string the user passed on the command line.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/cli/src/invoke-command.ts
**Line:** 419:431
**Comment:**
*Incorrect Variable Usage: Any `cron.*` scaffold request is silently rewritten to `cron.tick`, so the generated fixture can represent a different event type than the user asked for. Preserve the requested type (or emit a warning/error) instead of coercing it.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| } else if (a === '--cloud-url') { | ||
| cloudUrl = expectValue('--cloud-url', rest[++i]); | ||
| } else if (a === '--no-prompt') { | ||
| noPrompt = true; | ||
| } else if (a.startsWith('--')) { | ||
| throw new Error(`runs export: unknown flag "${a}"`); |
There was a problem hiding this comment.
Suggestion: The parser accepts --cloud-url <value> but not --cloud-url=<value>, so users using the common inline form will get a hard failure. Add the inline branch to keep behavior consistent with other commands and avoid avoidable CLI failures. [api mismatch]
Severity Level: Major ⚠️
- ❌ `runs export` rejects inline `--cloud-url=` usage.
- ⚠️ Diverges from other CLI commands' flag handling.Steps of Reproduction ✅
1. Invoke `agentworkforce runs export <runId> --cloud-url=https://cloud.example` so
`runRuns()` in `packages/cli/src/runs-command.ts:137-157` receives `args = ['export',
'<runId>', '--cloud-url=https://cloud.example']`.
2. `runRuns()` calls `parseRunsArgs()` at `packages/cli/src/runs-command.ts:43-101`; the
loop at lines 59-84 iterates over `'--cloud-url=https://cloud.example'`.
3. The parser only accepts the spaced form `a === '--cloud-url'` at line 73 and does not
have a `a.startsWith('--cloud-url=')` branch, unlike parsers in
`packages/cli/src/list-command.ts` which handle both `--cloud-url` and `--cloud-url=` for
deployment list and logs (see lines 205-208 and 251-254).
4. Because `'--cloud-url=https://cloud.example'` doesn't match the exact `--cloud-url`
form, it hits `else if (a.startsWith('--'))` at lines 77-78, throwing `runs export:
unknown flag "--cloud-url=https://cloud.example"`, which `runRuns()` surfaces as a usage
error instead of honoring the inline `--cloud-url=` format.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/cli/src/runs-command.ts
**Line:** 73:78
**Comment:**
*Api Mismatch: The parser accepts `--cloud-url <value>` but not `--cloud-url=<value>`, so users using the common inline form will get a hard failure. Add the inline branch to keep behavior consistent with other commands and avoid avoidable CLI failures.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| try { | ||
| payload = await requestJson<EnvelopeResponse>(url, ctx.token, 'runs export'); | ||
| matchedAgent = agent; | ||
| break; | ||
| } catch { | ||
| // 404 = not this agent's run (or auth/transient on this probe); | ||
| // keep scanning the remaining candidates. | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Suggestion: The probe loop swallows every request failure and keeps scanning, so auth/network/server errors are misreported as "run not found". Only 404 should be treated as "not this agent"; for other statuses, propagate the error immediately so users get the real failure cause. [logic error]
Severity Level: Critical 🚨
- ❌ `runs export` hides backend or auth failures as not-found.
- ⚠️ Users misdiagnose expired tokens or server outages.Steps of Reproduction ✅
1. Run `agentworkforce runs export <runId>` so the top-level CLI in
`packages/cli/src/cli.ts:73-77` dispatches to `runRuns()` in
`packages/cli/src/runs-command.ts:137-157` with arguments like `['export', '<runId>']`.
2. `runRuns()` calls `parseRunsArgs()` at `packages/cli/src/runs-command.ts:43-101` to
build `{ action: 'export', options }`, then calls `runRunsExport(parsed.options, io)` at
lines 151-153.
3. In `runRunsExport()` at `packages/cli/src/runs-command.ts:159-221`, the code builds a
list of candidate agents and enters the `for (const agent of candidates)` loop at lines
180-185, constructing an envelope URL per agent.
4. For each agent, it calls `requestJson<EnvelopeResponse>(url, ctx.token, 'runs export')`
inside the `try` block at lines 187-189; `requestJson()` (implemented in
`packages/cli/src/list-command.ts:409-418`) throws for any non-2xx response, including
401, 403, 500, and 404. The bare `catch { ... }` at
`packages/cli/src/runs-command.ts:191-195` swallows every such error and continues to the
next agent. If all probes fail (e.g. due to expired credentials causing 401 from every
agent), `payload` and `matchedAgent` remain null and the code throws a generic "run <id>
not found in workspace …" error at lines 198-203, misreporting authorization or server
failures as a missing run instead of surfacing the real backend error.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/cli/src/runs-command.ts
**Line:** 187:195
**Comment:**
*Logic Error: The probe loop swallows every request failure and keeps scanning, so auth/network/server errors are misreported as "run not found". Only 404 should be treated as "not this agent"; for other statuses, propagate the error immediately so users get the real failure cause.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| if (payload.captured === true && payload.envelope !== null && payload.envelope !== undefined) { | ||
| return { ok: true, fixture: `${JSON.stringify(payload.envelope, null, 2)}\n` }; |
There was a problem hiding this comment.
Suggestion: A captured envelope is accepted as long as it is non-null, but non-object payloads (e.g. string/number) will still be exported and later fail when passed to invoke --fixture. Validate that the envelope is an object before exporting to avoid generating unusable fixtures. [type error]
Severity Level: Major ⚠️
- ❌ `runs export` may emit non-object fixtures unusable by invoke.
- ⚠️ Breaks replay pipeline between cloud exports and simulations.Steps of Reproduction ✅
1. Assume the cloud envelope endpoint responds for some run with JSON like `{ "captured":
true, "envelope": "ok" }` (non-object envelope). `runRunsExport()` in
`packages/cli/src/runs-command.ts:159-221` fetches this via
`requestJson<EnvelopeResponse>()` at lines 187-189 and stores it in `payload`.
2. After finding a matching agent, `runRunsExport()` calls
`interpretEnvelopeResponse(payload, opts.runId, matchedAgent.deployedName)` at lines
206-207, passing the unvalidated `EnvelopeResponse` into `interpretEnvelopeResponse()`
defined at lines 227-251.
3. In `interpretEnvelopeResponse()` at `packages/cli/src/runs-command.ts:231-234`, the
first branch only checks `payload.captured === true` and `payload.envelope !== null &&
payload.envelope !== undefined`, without verifying that `envelope` is an object. For the
`"ok"` example, this branch returns `{ ok: true, fixture: "\"ok\"\n" }`, and
`runRunsExport()` writes that as the fixture file or stdout at lines 211-220.
4. When a user later replays the run with `agentworkforce invoke <persona> --fixture
exported.json`, `runInvokeWithOptions()` in `packages/cli/src/invoke-command.ts:299-360`
reads the fixture and calls `parseFixtureEnvelopes()` at lines 154-188. Since the fixture
text starts with a quote and not `{` or `[`, `parseFixtureEnvelopes()` falls through to
the final error at lines 186-188, throwing `invoke: fixture exported.json must be a JSON
envelope object, a JSON array, or NDJSON lines`. This shows that `runs export` can emit
fixtures that `invoke` cannot parse because `interpretEnvelopeResponse()` does not enforce
that `payload.envelope` is a JSON object envelope.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/cli/src/runs-command.ts
**Line:** 232:233
**Comment:**
*Type Error: A captured envelope is accepted as long as it is non-null, but non-object payloads (e.g. string/number) will still be exported and later fail when passed to `invoke --fixture`. Validate that the envelope is an object before exporting to avoid generating unusable fixtures.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished reviewing your PR. |
…nsient errors
reviewer-1's binding finding (gemini high, traced): the bare
catch{continue} laundered requestJson's 401 — which already carries the
actionable 'unauthorized. Run `agentworkforce login`' hint — into a
misleading 'run not found... check the run id'. The probe now continues
ONLY on a 404 (isProbeNotFound, exported + unit-tested with negative
controls) and rethrows everything else, which also makes the single
--agent candidate fail loud unconditionally. Same push: =-inline forms
for --workspace/--cloud-url (reviewer nit).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Fixed the PR issues I found. Changed:
Local verification:
All passed locally. |
…object envelopes codeant actionable pair (lead-triaged, reviewer-confirmed): (1) --scaffold cron.daily silently rewrote the fixture to cron.tick — a DIFFERENT event type than asked; the requested type is now preserved with a warning (the gateway emits cron.tick; the shim dispatches any cron.*), consistent with warn-don't-block. (2) interpretEnvelopeResponse accepted any non-null envelope, so a contract-violating string/number/array would export as a 'fixture' that fails later at invoke — the accept path now requires a JSON object, same never-fabricate stance, with string/number/array negative controls. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/invoke-command.ts`:
- Around line 277-288: The scaffold branch performs async I/O (scaffoldFixture,
writeFile, io.stdout/stderr) outside the guarded invoke error path, so failures
bypass the normalized invoke error handling; fix this by ensuring the scaffold
branch is executed inside the same try/catch used by runInvoke (or wrap the
scaffold I/O in a try/catch that rethrows the caught error) so that writeFile or
io.* rejections propagate to the existing invoke error handler and produce the
normalized "invoke: ..." error and exit code; locate the scaffold branch
referencing opts.scaffold, scaffoldFixture, writeFile, io.stdout/io.stderr and
either move it into the main invoke try block or add a small try { ... await
writeFile(...) / io.stdout(...) } catch (err) { throw err } to let the outer
handler process it.
In `@packages/cli/src/runs-command.ts`:
- Around line 191-194: The blanket catch in the candidate-probing loop swallows
all errors; change the catch in runs-command.ts so it inspects the thrown error
(e.g., err.status or err.response?.status) and only continues the loop when the
HTTP status is 404, otherwise rethrow the error; update the catch around the
probe/fetch call (the try that currently ends with "continue;") to differentiate
404 vs non-404 failures and rethrow non-404 exceptions so auth/network/server
errors are not mistaken for "run not found".
- Around line 71-74: The argument parser currently only handles split-value
flags for workspace and cloudUrl; update the branch that checks a ===
'--workspace' and a === '--cloud-url' to also accept inline forms by checking if
a startsWith('--workspace=') or startsWith('--cloud-url=') and extracting the
substring after '=' (fallback to calling expectValue('--workspace', rest[++i]) /
expectValue('--cloud-url', rest[++i]) when no '=' is present). Use the same
variables workspace and cloudUrl and reuse expectValue for the split form;
ensure you trim/validate the extracted value the same way as expectValue does so
both inline and split forms behave identically.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c3c19110-ccbf-44bd-a461-029722c3f26f
📒 Files selected for processing (10)
README.mdpackages/cli/src/cli.tspackages/cli/src/invoke-command.test.tspackages/cli/src/invoke-command.tspackages/cli/src/list-command.tspackages/cli/src/runs-command.test.tspackages/cli/src/runs-command.tspackages/runtime/src/envelope-fields.cloud.tspackages/runtime/src/shim.contract.test.tspackages/runtime/src/shim.ts
| if ('scaffold' in opts) { | ||
| const { fixture, warnings } = scaffoldFixture(opts.scaffold); | ||
| for (const warning of warnings) io.stderr(`warn: ${warning}\n`); | ||
| const text = `${JSON.stringify(fixture, null, 2)}\n`; | ||
| if (opts.outputPath) { | ||
| await writeFile(opts.outputPath, text, 'utf8'); | ||
| io.stderr(`fixture skeleton written to ${opts.outputPath}\n`); | ||
| } else { | ||
| io.stdout(text); | ||
| } | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
Handle scaffold write failures through the same invoke error path.
--scaffold does async output I/O outside the guarded invoke try/catch, so output write failures can reject runInvoke directly instead of emitting a normalized invoke: ... error and setting exit code 1.
Suggested patch
- if ('scaffold' in opts) {
- const { fixture, warnings } = scaffoldFixture(opts.scaffold);
- for (const warning of warnings) io.stderr(`warn: ${warning}\n`);
- const text = `${JSON.stringify(fixture, null, 2)}\n`;
- if (opts.outputPath) {
- await writeFile(opts.outputPath, text, 'utf8');
- io.stderr(`fixture skeleton written to ${opts.outputPath}\n`);
- } else {
- io.stdout(text);
- }
- return undefined;
- }
+ if ('scaffold' in opts) {
+ try {
+ const { fixture, warnings } = scaffoldFixture(opts.scaffold);
+ for (const warning of warnings) io.stderr(`warn: ${warning}\n`);
+ const text = `${JSON.stringify(fixture, null, 2)}\n`;
+ if (opts.outputPath) {
+ await writeFile(opts.outputPath, text, 'utf8');
+ io.stderr(`fixture skeleton written to ${opts.outputPath}\n`);
+ } else {
+ io.stdout(text);
+ }
+ process.exitCode = 0;
+ } catch (err) {
+ io.stderr(`invoke: ${err instanceof Error ? err.message : String(err)}\n`);
+ process.exitCode = 1;
+ }
+ return undefined;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ('scaffold' in opts) { | |
| const { fixture, warnings } = scaffoldFixture(opts.scaffold); | |
| for (const warning of warnings) io.stderr(`warn: ${warning}\n`); | |
| const text = `${JSON.stringify(fixture, null, 2)}\n`; | |
| if (opts.outputPath) { | |
| await writeFile(opts.outputPath, text, 'utf8'); | |
| io.stderr(`fixture skeleton written to ${opts.outputPath}\n`); | |
| } else { | |
| io.stdout(text); | |
| } | |
| return undefined; | |
| } | |
| if ('scaffold' in opts) { | |
| try { | |
| const { fixture, warnings } = scaffoldFixture(opts.scaffold); | |
| for (const warning of warnings) io.stderr(`warn: ${warning}\n`); | |
| const text = `${JSON.stringify(fixture, null, 2)}\n`; | |
| if (opts.outputPath) { | |
| await writeFile(opts.outputPath, text, 'utf8'); | |
| io.stderr(`fixture skeleton written to ${opts.outputPath}\n`); | |
| } else { | |
| io.stdout(text); | |
| } | |
| process.exitCode = 0; | |
| } catch (err) { | |
| io.stderr(`invoke: ${err instanceof Error ? err.message : String(err)}\n`); | |
| process.exitCode = 1; | |
| } | |
| return undefined; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/invoke-command.ts` around lines 277 - 288, The scaffold
branch performs async I/O (scaffoldFixture, writeFile, io.stdout/stderr) outside
the guarded invoke error path, so failures bypass the normalized invoke error
handling; fix this by ensuring the scaffold branch is executed inside the same
try/catch used by runInvoke (or wrap the scaffold I/O in a try/catch that
rethrows the caught error) so that writeFile or io.* rejections propagate to the
existing invoke error handler and produce the normalized "invoke: ..." error and
exit code; locate the scaffold branch referencing opts.scaffold,
scaffoldFixture, writeFile, io.stdout/io.stderr and either move it into the main
invoke try block or add a small try { ... await writeFile(...) / io.stdout(...)
} catch (err) { throw err } to let the outer handler process it.
|
Fixed the PR issues I found:
Validation run locally:
All completed successfully. |
|
✅ pr-reviewer applied fixes — committed and pushed Fixed the PR issues I found:
Validation run locally:
All completed successfully. |
|
Reviewed PR #192 and made targeted fixes. Fixed:
Validation run locally:
All completed successfully. |
|
Reviewed PR #192 and made targeted fixes. Fixed:
Validation run locally:
All completed successfully. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/invoke-command.ts (1)
282-293:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset
process.exitCodeon successful scaffold runs.Line 282 returns from the scaffold path without setting a success code. If
runInvokeis called multiple times in-process, a previous non-zeroprocess.exitCodecan leak into a successful--scaffoldexecution.Suggested patch
if ('scaffold' in opts) { const { fixture, warnings } = scaffoldFixture(opts.scaffold); for (const warning of warnings) io.stderr(`warn: ${warning}\n`); const text = `${JSON.stringify(fixture, null, 2)}\n`; if (opts.outputPath) { await writeFile(opts.outputPath, text, 'utf8'); io.stderr(`fixture skeleton written to ${opts.outputPath}\n`); } else { io.stdout(text); } + process.exitCode = 0; return undefined; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/invoke-command.ts` around lines 282 - 293, The scaffold branch returns early without clearing a prior non-zero exit status; inside the if ('scaffold' in opts) block (around the scaffoldFixture/ writeFile/ io.stdout logic), set process.exitCode = 0 just before the final return so a previous failed run doesn't leak into subsequent in-process successful --scaffold runs; update the code path that calls scaffoldFixture and returns undefined to reset process.exitCode to 0 before returning.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/cli/src/invoke-command.ts`:
- Around line 282-293: The scaffold branch returns early without clearing a
prior non-zero exit status; inside the if ('scaffold' in opts) block (around the
scaffoldFixture/ writeFile/ io.stdout logic), set process.exitCode = 0 just
before the final return so a previous failed run doesn't leak into subsequent
in-process successful --scaffold runs; update the code path that calls
scaffoldFixture and returns undefined to reset process.exitCode to 0 before
returning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d7056067-b65e-4bea-a77b-4bcf6374d862
📒 Files selected for processing (4)
packages/cli/src/invoke-command.test.tspackages/cli/src/invoke-command.tspackages/cli/src/runs-command.test.tspackages/cli/src/runs-command.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/cli/src/invoke-command.test.ts
- packages/cli/src/runs-command.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/runs-command.ts">
<violation number="1" location="packages/cli/src/runs-command.ts:282">
P2: `isProbeNotFound` uses regex matching on error message text instead of structured HTTP status, making probe retry/fail behavior brittle and potentially misclassifying non-404 errors.
(Based on your team's feedback about checking request errors by status instead of regex-parsing messages.) [FEEDBACK_USED].</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| * Exported for unit tests. | ||
| */ | ||
| export function isProbeNotFound(err: unknown): boolean { | ||
| return err instanceof Error && / failed: 404\b/.test(err.message); |
There was a problem hiding this comment.
P2: isProbeNotFound uses regex matching on error message text instead of structured HTTP status, making probe retry/fail behavior brittle and potentially misclassifying non-404 errors.
(Based on your team's feedback about checking request errors by status instead of regex-parsing messages.) .
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/runs-command.ts, line 282:
<comment>`isProbeNotFound` uses regex matching on error message text instead of structured HTTP status, making probe retry/fail behavior brittle and potentially misclassifying non-404 errors.
(Based on your team's feedback about checking request errors by status instead of regex-parsing messages.) .</comment>
<file context>
@@ -249,3 +271,13 @@ export function interpretEnvelopeResponse(
+ * Exported for unit tests.
+ */
+export function isProbeNotFound(err: unknown): boolean {
+ return err instanceof Error && / failed: 404\b/.test(err.message);
+}
</file context>
|
Reviewed PR #192 and made one fix: Local validation passed:
|
|
✅ pr-reviewer applied fixes — committed and pushed Reviewed PR #192 and made one fix: Local validation passed:
|
1 participant
User description
Summary
#189 P2, consuming cloud#1841 (merged): fixture discoverability for
invoke.agentworkforce runs export <runId> [--fixture event.json] [--agent <selector>]— pulls the byte-exact delivered envelope from cloud'sGET /runs/:runId/envelopeand writes the single-envelope fixtureinvokeconsumes. The fixture IS cloud's normalized output, so replay can't drift. Agent-scoped endpoint → without--agent, every workspace agent is probed for the run id (bounded by agent count). Omitted/pre-capture runs get actionable errors pointing at--scaffold; a contract-violatingcaptured:true+envelope:nullresponse is refused, never fabricated (interpretEnvelopeResponse, exported + unit-tested). Auth/fetch reuses thedeployments list|logshelpers (now exported fromlist-command.ts).agentworkforce invoke --scaffold <type>— cold-start skeleton: frame filled (name/cronforcron.tick),resourcean explicit TODO hole for provider events (per the issue: its shape is adapter-normalization +buildEnvelopeterritory and must not be hand-guessed).<type>validated againstKNOWN_TRIGGER_CATALOG, warn-don't-block.RawGatewayEnvelopegains the 5 missing cloud fields (provider,eventType,deliveryId,paths,resumeContext);RAW_GATEWAY_ENVELOPE_FIELDSis compile-checked against the type (satisfies keyof);envelope-fields.cloud.tsis the checked-in copy of cloud's mergedENVELOPE_FIELDSanchor (source + update procedure in the header);shim.contract.test.tsasserts every cloud field is documented AND that a full cloud-shaped envelope still shims/dispatches. Cloud's pin test (cloud#1851) names this copy in its failure message — drift fails CI on whichever side moves first.Tests
Runtime 52/52 (3 new contract tests) · CLI 240/240 (11 new: parse contracts, captured/omitted/pre-capture/violating-response interpretation, scaffold frame + TODO hole + warn-don't-block + e2e through
runInvoke). Fullpnpm run checkgreen (fresh worktree, realpnpm install, workspace deps built — no hand-linking).Issue mapping
runs export— real fire → replayable fixture (primary path)--scaffold— cold-start skeleton with catalog validationRawGatewayEnvelopeRefs #189, AgentWorkforce/cloud#1841
🤖 Generated with Claude Code
CodeAnt-AI Description
Export real run envelopes and scaffold fixture files from the CLI
What Changed
runs export <runId>to download the exact envelope delivered to a run and save it as a replayableinvokefixture, or print it to stdoutinvoke --scaffold <type>to generate a starter fixture without needing a persona or a real run, with a placeholderresourcesection for provider eventsImpact
✅ Replay fixtures that match real runs✅ Faster fixture creation before the first live event✅ Clearer guidance for missing or oversized run envelopes💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.